fish completions: fix double-evaluation of commandline #2095
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
krobelus
commented
Jan 14, 2024
fish_completions.go
Outdated
| @@ -45,7 +45,7 @@ function __%[1]s_perform_completion | |||
| __%[1]s_debug "Starting __%[1]s_perform_completion" | |||
|
|
|||
| # Extract all args except the last one | |||
| set -l args (commandline -opc) | |||
| set -l args (commandline -opc | string escape) | |||
There was a problem hiding this comment.
cc @marckhouzam
FWIW the blame for this mistake is on me, I made the suggestion to use it
|
@krobelus, do you know what needs to be done for this PR to be merged and included in the next release? |
|
The API to avoid this downside while still fixing the issue at hand has been merged in fish-shell/fish-shell#10212, which will be in the upcoming fish release. |
We capture the commandline tokens using fish's "commandline --tokenize" (-o). Given a commandline of some-cobra-command 'some argument $(123)' "$HOME" into three arguments while removing the quotes some-cobra-command some argument $(123) $HOME Later we pass "some argument $(123)" without quotes to the shell's "eval". This is wrong and causes spurious evaluation of the parenthesis as command substitution. Fix this by escaping the arguments. The upcoming fish 3.8 has a new flag, "commandline -x" which will expand variables like $HOME properly, see fish-shell/fish-shell#10212 Use that if available. Reproduce the issue by pasting the completion script at fish-shell/fish-shell#10194 (comment) to a file "grafana-manager.fish" and running function grafana-manager; end source grafana-manager.fish Then type (without pressing Enter) grafana-manager public-dashboards delete --organization-id 3 --dashboard-name "k8s (public)" <TAB> Fixes fish-shell/fish-shell#10194
krobelus
force-pushed
the
fix-fish-completions
branch
from
eabe4e5
to
4e07448
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We capture the commandline tokens using fish's "commandline --tokenize" (-o).
That function turns
into two arguments while removing the quotes
Later we pass "some argument $(123)" without quotes to the shell's
"eval". This is wrong and causes spurious evaluation of the parenthesis
as command substitution.
Fix this by escaping the arguments.
The downside of this change is that things like "$HOME" or "~" will
no longer be escaped. Changing this requires changes in fish, which
I'm working on.
Reproduce the issue by pasting the completion script at
fish-shell/fish-shell#10194 (comment)
to a file "grafana-manager.fish" and running inside
fishThen type (without pressing Enter)
Fixes fish-shell/fish-shell#10194